-
Notifications
You must be signed in to change notification settings - Fork 641
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add type annotations to Simulation
class
#1919
Conversation
As a general principle, this seems fine, but one has to be very carefully about overly restricting the types, because then you lose flexibility — Python potentially has a wide variety of types that can, for example, represent a real number (and which are convertible by SWIG to |
Note also that this requires Python 3.5; what's the minimum version of Python that we currently support? |
The GitHub actions CI uses Python 3.6 and 3.9. The Conda packages for the latest version 1.22 use Python 3.7 - 3.10. Python 3.5 was released in September 2015 which is fairly outdated. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1919 +/- ##
==========================================
+ Coverage 73.15% 73.16% +0.01%
==========================================
Files 17 17
Lines 4924 4926 +2
==========================================
+ Hits 3602 3604 +2
Misses 1322 1322
|
Simulation
class
So int is a subtype of float in this type system? |
Is there a way to work with earlier Python releases? Requiring Python 3.7 for this is a bit annoying, since that is less than 4 years old. I don't mind requiring Python 3.5. |
Even though Python is a dynamically typed language, type annotations (or hints) can be used for static analysis using tools such as mypy or pytype. It would be useful to eventually add type annotations to all Python classes and functions in the API. This way, we could add a static analyzer (including a linter) as a GitHub Action as part of the CI.
One anomaly with the current setup is that using the automated script
generate_api_doc.py
to update the user manual markdown pagePython_User_Interface.md
which is generated from the docstrings shows the module filename as part of the class name. For example, theSimulation
constructor includes:which appears in the user manual as:
It would be cleaner if the module filename
meep.simulation
did not appear. EDIT: Actually, this can be fixed by adding: